Trac 64389: Refine hoisted stylesheet ordering in classic themes#10875
Trac 64389: Refine hoisted stylesheet ordering in classic themes#10875westonruter wants to merge 24 commits intoWordPress:trunkfrom
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
…ut comments and then empty STYLE tags
This comment was marked as duplicate.
This comment was marked as duplicate.
There was a problem hiding this comment.
Pull request overview
Refines how global-styles is hoisted into the document <head> for classic themes when block assets are loaded on-demand, addressing cases where no block styles (and no classic-theme-styles) provide a reliable insertion point.
Changes:
- Enqueue a
wp-global-styles-placeholderinline style duringwp_enqueue_scriptsfor classic themes with on-demand loading, sowp_hoist_late_printed_styles()can reliably replace it with the footer-printedglobal-stylesin the<head>. - Update
wp_hoist_late_printed_styles()to detect and replace the new global-styles placeholder bookmark. - Expand PHPUnit coverage to include “Elementor-like” scenarios (theme.json present + no block content + dequeued block library), and adjust expected ordering accordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/wp-includes/script-loader.php |
Adds/enables a global-styles placeholder in classic+on-demand mode and replaces it during hoisting to stabilize <head> ordering. |
tests/phpunit/tests/template.php |
Updates hoisting tests to include content parameterization and new scenarios/expectations around placeholder-driven ordering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
This new placeholder approach may be relevant the hoisted placement of other styles. But I'd really love to get more reproduction steps for any issues people are experiencing. |
…nlined core block styles
…into trac-64389
There was a problem hiding this comment.
The changes in this file I don't intend to commit as part of this PR. They belong in a PR for Core-64238. Nevertheless, I added the changes here to ensure all of the new code introduced as part of wp_hoist_late_printed_styles() is fully passing PHPStan rule level 8.
…action()/has_filter()
…lder where block styles are enqueued
| /* | ||
| * Global styles should be printed in the head for block themes, or for classic themes when loading assets on | ||
| * demand is disabled, which is the default. | ||
| * The footer should only be used for classic themes when loading assets on demand is enabled. | ||
| /** | ||
| * Global styles should be printed in the HEAD for block themes, or for classic themes when loading assets on | ||
| * demand is disabled (which is no longer the default). | ||
| * | ||
| * See https://core.trac.wordpress.org/ticket/53494 and https://core.trac.wordpress.org/ticket/61965. | ||
| * @link https://core.trac.wordpress.org/ticket/53494 | ||
| * @link https://core.trac.wordpress.org/ticket/61965 | ||
| */ | ||
| if ( | ||
| ( $is_block_theme && doing_action( 'wp_footer' ) ) || | ||
| ( $is_classic_theme && doing_action( 'wp_footer' ) && ! $assets_on_demand ) || | ||
| ( $is_classic_theme && doing_action( 'wp_enqueue_scripts' ) && $assets_on_demand ) | ||
| doing_action( 'wp_footer' ) && | ||
| ( | ||
| $is_block_theme || | ||
| ( $is_classic_theme && ! $assets_on_demand ) | ||
| ) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| /** | ||
| * The footer should only be used for classic themes when loading assets on demand is enabled. This is now the | ||
| * default with the introduction of hoisting late-printed styles (via {@see wp_load_classic_theme_block_styles_on_demand()}). | ||
| * So even though the main global styles are not printed here in the HEAD for classic themes with on-demand asset | ||
| * loading, a placeholder for the global styles is still enqueued. Then when {@see wp_hoist_late_printed_styles()} | ||
| * processes the output buffer, it can locate the placeholder and inject the global styles from the footer into the | ||
| * HEAD. | ||
| * | ||
| * @link https://core.trac.wordpress.org/ticket/64099 | ||
| */ | ||
| if ( $is_classic_theme && doing_action( 'wp_enqueue_scripts' ) && $assets_on_demand ) { | ||
| if ( has_action( 'wp_template_enhancement_output_buffer_started', 'wp_hoist_late_printed_styles' ) ) { | ||
| wp_register_style( 'wp-global-styles-placeholder', false ); | ||
| wp_add_inline_style( 'wp-global-styles-placeholder', ':root { --wp-internal-comment: "Placeholder for wp_hoist_late_printed_styles() to replace with the global-styles printed at wp_footer." }' ); | ||
| wp_enqueue_style( 'wp-global-styles-placeholder' ); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
This needs to be patched in Gutenberg's gutenberg_enqueue_global_styles() as well: https://github.com/WordPress/gutenberg/blob/d37cda8aadb242d506451e1b5e649737d59224e3/lib/script-loader.php#L19-L36
There was a problem hiding this comment.
Note how Gutenberg unhooks wp_enqueue_global_styles() from running in favor of gutenberg_enqueue_global_styles():
There was a problem hiding this comment.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( $is_classic_theme && doing_action( 'wp_enqueue_scripts' ) && $assets_on_demand ) { | ||
| if ( has_action( 'wp_template_enhancement_output_buffer_started', 'wp_hoist_late_printed_styles' ) ) { | ||
| wp_register_style( 'wp-global-styles-placeholder', false ); | ||
| wp_add_inline_style( 'wp-global-styles-placeholder', ':root { --wp-internal-comment: "Placeholder for wp_hoist_late_printed_styles() to replace with the global-styles printed at wp_footer." }' ); |
There was a problem hiding this comment.
The placeholder CSS currently sets a :root custom property (--wp-internal-comment). If, for any reason, the output-buffer replacement doesn’t run, this will leak into the final page CSS and may collide with user-defined custom properties. Consider using a CSS comment (or another fully inert construct) as the inline placeholder content so it has no runtime styling side effects.
| wp_add_inline_style( 'wp-global-styles-placeholder', ':root { --wp-internal-comment: "Placeholder for wp_hoist_late_printed_styles() to replace with the global-styles printed at wp_footer." }' ); | |
| wp_add_inline_style( 'wp-global-styles-placeholder', '/* Placeholder for wp_hoist_late_printed_styles() to replace with the global-styles printed at wp_footer. */' ); |
There was a problem hiding this comment.
A style rule like this is intentional, because I didn't want some optimization logic to try stripping out styles, resulting in the inline style being removed.
| */ | ||
| if ( ! wp_is_block_theme() && has_action( 'wp_template_enhancement_output_buffer_started', 'wp_hoist_late_printed_styles' ) ) { | ||
| wp_register_style( 'wp-block-styles-placeholder', false ); | ||
| wp_add_inline_style( 'wp-block-styles-placeholder', ':root { --wp-internal-comment: "Placeholder for wp_hoist_late_printed_styles() to replace with the block styles printed at wp_footer." }' ); |
There was a problem hiding this comment.
The placeholder CSS currently sets a :root custom property (--wp-internal-comment). If, for any reason, the output-buffer replacement doesn’t run, this will leak into the final page CSS and may collide with user-defined custom properties. Consider using a CSS comment (or another fully inert construct) as the inline placeholder content so it has no runtime styling side effects.
| wp_add_inline_style( 'wp-block-styles-placeholder', ':root { --wp-internal-comment: "Placeholder for wp_hoist_late_printed_styles() to replace with the block styles printed at wp_footer." }' ); | |
| wp_add_inline_style( 'wp-block-styles-placeholder', '/* Placeholder for wp_hoist_late_printed_styles() to replace with the block styles printed at wp_footer. */' ); |
There was a problem hiding this comment.
A style rule like this is intentional, because I didn't want some optimization logic to try stripping out styles, resulting in the inline style being removed.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Trac ticket: https://core.trac.wordpress.org/ticket/64389
When a theme has a
theme.json, then it will not emit theclassic-theme-stylesstylesheet. When no other block styles printed either, as in the case of Elementor with the Hello Elementor theme, then the existing logic inwp_hoist_late_printed_styles()does not have a reference point (from printed block styles) to know where it should inject theglobal-styles. So, this PR makes the hoistingglobal-stylesmuch more robust by enqueueing a placeholder inline style which then gets replaced during processing of the template enhancement output buffer. This also allows for removing some convolutedifstatements to conditionally inject global styles around other styles.The same goes for where non-core block styles get printed. Previously the logic here would attempt to locate the
classic-theme-stylesand then insert the block styles after that style. But if the style was not enqueued, then it would have fall back to trying to insert them after elsewhere. This, again, is simplified by printing a placeholder inline style at the point where block styles normally get inserted. This placeholder style in theHEADis replaced with the styles rendered in the footer.The placeholder inline styles are simply removed if there are no appropriate styles to hoist there.
How I tested:
gutenberg_enqueue_global_styles()to correspond with the modifiedwp_enqueue_global_styles()in this PR. Or else, have Sync changes fromwp_enqueue_global_styles()to Gutenberg override gutenberg#76127 checked out.add_filter( 'wp_should_output_buffer_template_for_enhancement', '__return_true' );trunkeliminated the underline from links (probably Remove link underline style from default theme.json gutenberg#74901), so add this plugin code to restore the previous underline to reproduce the bug with the cascade:STYLE#global-styles-inline-cssis printed at the end ofHEAD, far afterLINK#hello-elementor-cssSTYLE#global-styles-inline-csselement is printed afterSTYLE#wp-emoji-styles-inline-cssand beforeLINK#hello-elementor-css.Fixing Cascade for
wp-block-libraryadded inline stylesThis PR also fixes (via 48eb4f6) the cascade for when inline styles are added to
wp-block-library. When the combined block library is enqueued, theme authors would expect any inline style to appear in the cascade after any core block styles. However, in 6.9 this was no longer the case as the separate block styles would get inserted after any suchwp-block-libraryinline styles. This is fixed now so that the inline styles forwp-block-libraryget after any separate core block styles.For example, consider this plugin code:
Before 6.9, in a classic theme where combined block library is loaded, this would result in the following styles printed (e.g. in Twenty Twenty-One):
In 6.9, however, when separate block styles are loaded on demand, the lime color style appears before the block style for the Separator block:
With the changes in this PR, the correct order placement in the cascade is better preserved:
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.